ADFA-2707 | Model-driven open-file tool + reliable local-LLM agent loop#50
ADFA-2707 | Model-driven open-file tool + reliable local-LLM agent loop#50jatezzz wants to merge 3 commits into
Conversation
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
There was a problem hiding this comment.
Claude Code Review
This repository is configured for manual code reviews. Comment @claude review for a one-time review, or @claude review always to subscribe this PR to a review on every future push.
Tip: disable this comment in your organization's Code Review settings.
hal-eisen-adfa
left a comment
hal-eisen-adfa
left a comment
There was a problem hiding this comment.
Code review — 16 findings (correctness-focused, xhigh effort)
Reviewed 68a6f98 against the PR base refactor/ADFA-4589-ai-hackathon-plugin (not main) across 10 finder angles plus direct source verification. Inline comments below, ranked roughly by severity.
Gate-worthy: (1) PathGuard sandbox regresses to accept-all when the project root falls back to /; (2) the local-model GBNF grammar makes 7 registered tools structurally uncallable; (3) Clear-Chat history race; (4) session-wide lastToolResult mislabels replies as "action failed"; (5) open_file arg-remap gap; (6) grammar permits raw newlines → org.json drops the call.
Then a concurrency/cancellation tier (StringBuilder streaming race, no single-flight on the LLamaAndroid singleton, Gemini/local cancellation not actually aborting native/HTTP work) and agent-loop edge cases (co-emitted respond, REPEATED false-abort), plus JNI null-checks.
Verified NOT problems: the JNI/API boundary is consistent (all new_grammar_sampler/setGrammar signatures, State.Loaded.model, LlmConfig.extraParams, ToolResult fields, R.string.* all line up); the adapter/fragment UX issues some reviewers might flag are on unchanged lines outside this diff. No CLAUDE.md convention violation.
Two comments (Executor.kt, ToolCallExtractor.kt) anchor to the nearest in-diff line in the same function; the true line is noted in the body.
Plugin review finding — reflection into ai-core internals (rubric 6.5)
// AiSettingsViewModel.kt:258-259
val method = geminiBackend.javaClass.getMethod("listModels")
val futureResult = method.invoke(geminiBackend)
It's moderate severity — Suggested fix: add (Note: this file isn't part of the current PR diff, so posting as a general comment rather than inline.) |
3d33b7a to
b939ff5
Compare
a612e55 to
6400ebd
Compare
6389885 to
8c6c2d3
Compare
…agent loop Add an open_file tool exercising the real agent path. Make the text loop dependable on weak local models: project-anchored path resolution with basename/slash-prefixed fallback, anti-fabrication prompts, honest failure reporting, stop-after-success, and repeated-call/max-iteration backstops. Also: streaming cancellation (Local+Gemini), IDE-crash fixes, and tests.
Trim the local system prompt (prefill was the bottleneck), stop the model running past its reply, and gate per-token native logging under NDEBUG. Also reset _history on new/switched sessions so a fresh chat no longer answers the previous conversation's last message.
Add model warm-up + native longest-common-prefix KV-cache reuse; repair the tool-call grammar, PathGuard sandbox, agent loop, backend cancellation and JNI.
8c6c2d3 to
fbc0de3
Compare
2 participants
Description
Adds an
open_filetool to the AI Assistant plugin that exercises the real agent path (model requests a tool → agent loop executes it → file opens in the IDE editor), and hardens the text-based agent loop so it behaves reliably onweak local models.
The core problem this fixes: path resolution previously relied on the
project.dirsystem property (never set) and fell back touser.dir, which on Android is"/". Every relative path (e.g..gitignore) resolved against the filesystem root and was rejected as "outside the project directory," so the tool never worked. Resolution is now anchored to the actual open project, with basename/slash-prefixed fallbacks so a hallucinated bare filename still lands on the right file, or reports an honest, disambiguable failure.Details
open_filetool + path resolution (PathGuard,OpenFileHandler)IdeProjectService.currentProject.rootDir, queried lazily so a mid-session project switch is picked up automatically; legacy system-property fallbacks retained.PathGuard.resolve()returns a sealedResolution(Resolved/Ambiguous/NotFound/Escaped) so handlers can act or explain the miss.findByName()resolves a bare filename to its real path by walking the project root (case-insensitive), skipping generated/noise dirs (build,.git,.gradle,.idea,node_modules,.cxx) and hidden dirs, with a result cap.OpenFileHandleropens the editor tab on the main dispatcher (execute()runs on IO) and surfaces ambiguous matches back to the model instead of a stack trace.Reliable local-LLM agent loop (
AgentLoop,ChatViewModel)ToolCallExtractorwith a dedicatedAgentLoop.Streaming cancellation + crash fixes (
ai-core)CancellableBackend,LLamaAndroid,llama-android.cpp), plus IDE-crash fixes.Tests
AgentLoopTest,ExecutorTest,ToolCallExtractorTest,ListFilesHandlerTest,OpenFileHandlerTest,PathGuardTest(~1,000 lines of test coverage).document_5116333375648434041.mp4
Ticket
ADFA-2707
Observation
ai-core/libs/v8/llama-v8-release.aarandllama-api.jarare rebuilt binaries (native cancellation support), regenerated viaai-core/scripts/rebuild-llama-aar.sh.